GH-23686: SOAP: redact more headers by default, add keep_headers option - #23698

@php/release-managers-86 can you please let me know if this needs to be in by the branch cut? I'm unsure if this counts as a new feature that would be blocked by the feature freeze - it adds more options, but the motivation is to allow opting out of the bug fix

@mbeccati and I agree this can go into 8.6

This should be ready for review, ideally we should merge it in the next two weeks so that it is in RC3

Okay

• rebased
• ext/soap: fix leak when load_wsdl_ex() fails to load #24066 was merged, so that commit no longer needed to be cherry pick here for this branch to not have memory leaks
• fixed proxy authorization searching

hopefully should be ready for another round of review

If I'm reading the suggestions correctly, they are to handle headers that are stored as arrays properly - but the tests suggests that they are already handled?

Looks like this is because get_sdl() uses http_context_headers() which converts things from an array to a string when filtering out known headers, and that is only triggered if soap wants to add its own headers, able to trigger the failure with:

$context = stream_context_create([
  'http' => [
    // having a protocol here means that SOAP won't add it itself
    'protocol_version' => 1.1,
    'header' => [
      "X-Authorization: This should be kept",
      "Authorization: This should be removed",
      "X-Proxy-Authorization: This should also be kept",
      "Proxy-Authorization: This should also be removed",
      "X-Cookie: Last one to keep",
      "Cookie: Last one to remove",
    ]
  ],
]);

good catch - I hadn't addressed array headers originally since on #23686 I was told that they could be ignored, but you're right that we should probably address them, done

Regarding line separators: I was going based on CRLF, but RFC 9112 says (https://www.rfc-editor.org/info/rfc9112/#section-2.2)

Although the line terminator for the start-line and fields is the sequence CRLF, a recipient MAY recognize a single LF as a line terminator and ignore any preceding CR.

A sender MUST NOT generate a bare CR (a CR character not immediately followed by LF) within any protocol elements other than the content. A recipient of such a bare CR MUST consider that element to be invalid or replace each bare CR with SP before processing the element or forwarding the message.

so I'll go just by LF

Rebased and added NEWS/UPGRADING, once CI passes I'll merge this locally so that commits stay signed

Failure in FREEBSD is unrelated and pre-existing, merging

Tests related to this PR fail on nightly CI: https://github.com/php/php-src/actions/runs/41260026425/job/124711397763

Tests related to this PR fail on nightly CI: https://github.com/php/php-src/actions/runs/41260026425/job/124711397763

Hmm, and it isn't new, this seems to have been failing the LINUX_X64_VARIATION_DEBUG_ZTS job at least for a few days - the actual expected output comes out, but there is also

  005+ SoapFault: SOAP-ERROR: Parsing WSDL: Couldn't load from 'http://localhost:35921' : failed to load external entity "http://localhost:35921"
  006+ 

which likely comes from the check_headers.inc logging when the constructor throws

Coming from matrix.php ['name' => '_VARIATION', 'asan' => false, 'debug' => true, 'repeat' => false, 'variation' => true, 'zts' => true],

Failing tests are:

• ext/soap/tests/gh23686/import-repeated-authorization.phpt
• ext/soap/tests/gh23686/import-repeated-cookies.phpt
• ext/soap/tests/gh23686/import-repeated-proxy-auth.phpt
• ext/soap/tests/gh23686/schema-repeated-cookies.phpt
• ext/soap/tests/gh23686/schema-repeated-proxy-auth.phpt
• ext/soap/tests/gh23686/schema-repeated-authorization.phpt

I was able to reproduce this with ./configure --enable-option-checking=fatal --enable-gd --with-zip --enable-soap --enable-mbstring --with-curl --with-openssl --enable-zend-test --enable-dl-test=shared --with-sodium --enable-werror 'CFLAGS=-DZEND_RC_DEBUG=1 -DPROFITABILITY_CHECKS=0 -DZEND_VERIFY_FUNC_INFO=1 -DZEND_VERIFY_TYPE_INFERENCE' --enable-zend-max-execution-timers --enable-debug 'CXXFLAGS=-DZEND_VERIFY_TYPE_INFERENCE -D_GLIBCXX_ASSERTIONS' --enable-zts and sapi/cli/php run-tests.php -P -q -d zend_test.observer.enabled=1 -d zend_test.observer.show_output=0 -d opcache.jit=disable -d opcache.protect_memory=1 -d opcache.jit_buffer_size=64M -j4 -g FAIL,BORK,LEAK,XLEAK --no-progress --offline --show-diff --show-slow 1000 --set-timeout 120 ext/soap/tests/gh23686/, let me try to reduce it

Actually just sapi/cli/php run-tests.php ext/soap/tests/gh23686/ is enough, so this is something from the ./configure command

Okay, minimized the configuration:

Fail:
./configure --enable-soap --with-curl --with-openssl 'CFLAGS=-DZEND_RC_DEBUG=1' --enable-debug

Pass:
./configure --enable-soap --with-curl --with-openssl --enable-debug

Okay, so the error occurs the second time the SoapClient is constructed, and it doesn't matter which soap version is used for either the first or second construct, testing with SOAP_1_1 then SOAP_1_2, 1.2 then 1.1, 1.1 twice, and 1.2 twice, all fail

The second time through,

Unfortunately, I was unable to connect GDB with the libxml2 debug symbols, so I couldn't figure out why this is the case, and how ZEND_RC_DEBUG is at all related

Okay, so if in GDB we break on soap_xmlParseFile it gets called 3 times:

For the first SoapClient constructor, the original URL and then the one to import
For the second SoapClient constructor, the original URL <- this is the one that fails

E.g. http://localhost:39679, then http://localhost:35895/index.php, then http://localhost:39679 again

At the start, before the first soap_xmlParseFile runs, I can manually in the container curl repeatedly to both addresses
After the first soap_xmlParseFile runs, trying to curl that same original URL says curl: (7) Failed to connect to localhost port 45273 after 0 ms: Couldn't connect to server

Hacking the php_cli_server.inc to put the CLI output in an easier place to debug shows that

php: Zend/zend_types.h:829: uint32_t zend_gc_addref(zend_refcounted_h *): Assertion `(zval_gc_flags((p)->u.type_info) & ((1<<7)|(1<<8))) != (1<<7)' failed.

and that presumably causes the server to stop

This is actually unrelated to the SoapClient stuff, I was able to reproduce with by manually running curl with the temporary server that is being used for the tests:

root@3b54b073fb19:/var/www/html/repeated-headers# cat index.php 
<?php

echo "Testing\n";

root@3b54b073fb19:/var/www/html/repeated-headers# /usr/src/php86/sapi/cli/php -d error_include_args=0 -t /var/www/html/repeated-headers/ -n -S localhost:0
[Sun Nov  1 17:32:00 2026] PHP 8.6.0-dev Development Server (http://localhost:33929) started
[Sun Nov  1 17:32:08 2026] [::1]:53820 Accepted
[Sun Nov  1 17:32:08 2026] [::1]:53820 [200]: GET /
[Sun Nov  1 17:32:08 2026] [::1]:53820 Closing
php: /usr/src/php86/Zend/zend_types.h:849: uint32_t zend_gc_delref(zend_refcounted_h *): Assertion `(zval_gc_flags((p)->u.type_info) & ((1<<7)|(1<<8))) != (1<<7)' failed.
Aborted (core dumped)

and in another shell

root@3b54b073fb19:/var/www/html# curl -H "Cookie: Foo" -H "cOOKIE: Bar" http://localhost:33929
Testing

would the following solve the (main) issue ?

diff --git a/sapi/cli/php_cli_server.c b/sapi/cli/php_cli_server.c
--- a/sapi/cli/php_cli_server.c
+++ b/sapi/cli/php_cli_server.c
@@ -1701,6 +1701,7 @@ static void php_cli_server_client_save_header(php_cli_server_client *client)
        } else {
                zend_string *curval = Z_STR_P(entry);
                zend_string *newval = zend_string_safe_alloc(1, ZSTR_LEN(curval),  ZSTR_LEN(client->current_header_value) + 2, /* persistent */true);
+               GC_MAKE_PERSISTENT_LOCAL(newval);

                memcpy(ZSTR_VAL(newval), ZSTR_VAL(curval), ZSTR_LEN(curval));
                memcpy(ZSTR_VAL(newval) + ZSTR_LEN(curval), ", ", 2);

@devnexen I haven't tested that, but certainly that is an issue fix even if it doesn't fix the case found by Daniel, so please PR that 🙂